Skip to content

Fix status bar custom board preferences disappearing #11200

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jan 26, 2021

Conversation

magedrifaat
Copy link
Contributor

This commit fixes #11164

The cause of the bug was that although EditorLineStatus' "paintComponent" function correctly calls "getBoardPreferences" which accounts for any custom board preferences, Editor's "onBoardOrPortChange" incorrectly calls "getTargetBoard" which only account for the board name.

That's why the board settings appear at first but any change to the board settings makes them disappear.
My fix was to simply call "getBoardPreferences" in Editor's "onBoardOrPortChange" just like the implementation in EditorLineStatus.

All Submissions:

  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same update/change?

Changes to Core Features:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you successfully ran tests with your changes

Copy link
Collaborator

@matthijskooijman matthijskooijman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've noticed this issue before, good to see it fixed.

Looking at the code, this looks like a reasonable fix. Haven't tested, though.

One thing that might improve this could be to remove the duplicate code entirely, for example by adding a updateBoardAndPort() method to EditorLineStatus that can be called from both onBoardOrPortChange and paintComponent (the latter within the existing if).

@magedrifaat
Copy link
Contributor Author

I've noticed this issue before, good to see it fixed.

Looking at the code, this looks like a reasonable fix. Haven't tested, though.

One thing that might improve this could be to remove the duplicate code entirely, for example by adding a updateBoardAndPort() method to EditorLineStatus that can be called from both onBoardOrPortChange and paintComponent (the latter within the existing if).

Thanks for the feedback.

The solution you are describing seems like the better option, but the reason I leaned towards this "dirty" fix is because having dealt with Arduino IDE code before, I found that keeping modifications to a minimum usually produces less bugs in the long run.

That being said, I think the reason this bug existed in the first place is because of the duplicated code, the feature was implemented in one place but not the other. So it makes more sense to get rid of the root of the problem to avoid it happening again. I'll just have to be more careful and mind any other "board changes" that might be happening somewhere else.

I'll try it out and update this pull request with the new fix.

@magedrifaat
Copy link
Contributor Author

This is turned out easier than I expected as I found that only the Editor deals with EditorLineStatus.

As per the contribution guidelines, I rebased the commit and made a force push in the fork to update the pull request instead of adding new commits.

Copy link
Collaborator

@matthijskooijman matthijskooijman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks perfect to me!

@facchinm
Copy link
Member

LGTM!

@facchinm facchinm added this to the Release 1.8.14 milestone Jan 26, 2021
@cmaglie cmaglie merged commit b2265ce into arduino:master Jan 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Satus bar is not properly updated
4 participants